-
Couldn't load subscription status.
- Fork 39
feat: Support globe view #908
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@kylebarron The general approach seems quite comprehensive. I’d suggest implementing the different modes more progressively, as this will likely require some refactoring on the JS side. In hindsight, adding XState wasn’t the best choice on my part, it makes state management a bit harder to evolve. I’ll open a ticket to propose an alternative. I think we can still address this technical debt while it’s not too complex, but not on this PR. |
|
I think despite what are a bunch of changes here on the Python side, the JS side only needs to change to support two modes: One with mapbox overlay and one without. We can also merge this PR or something similar without exposing the classes to users, so we can keep developing on the JS side |
|
@kylebarron sounds good. Now that we merged the Playwright PR we can add specs to ensure JS features continue to work as expected. |
…lay (#921) As described in the [upstream deck.gl docs](https://deck.gl/docs/developer-guide/base-maps/using-with-maplibre), there are three ways to support using deck.gl with Maplibre: _interleaved_, _overlaid_, and _reverse-controlled_. The first two are supported via `MapboxOverlay` with a prop `interleaved: true|false`, while the latter is implemented by having Maplibre be a child of the deck.gl Map. There are worthwhile reasons to support all of these modes. We need to use interleaved or overlaid to support globe view, while reverse-controlled better supports multiple deck.gl views. This PR refactors the map component in `index.tsx` into two separate React components: a deck.gl-first renderer (i.e. "reverse-controlled") and a MapboxOverlay renderer. The idea is that this will pair with #908 to give users more control over various ways of rendering maps. This is backwards-compatible because we default to reverse-controlled, the existing default. Closes #890, closes #437, for #886, for #718
A small refactor to make it easier to read `_map.py` by removing the HTML export functionality, moving that to `_html_export.py`. This was extracted from #908, see https://github.com/developmentseed/lonboard/pull/908/files#r2414276602
|
The basemap refactor was extracted to #935 |
| async loadSubModels() { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was unused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for globe view rendering in lonboard by introducing a Views abstraction layer that maps to deck.gl's view system. The implementation allows users to specify different view types (Globe, Map, Orbit, etc.) and automatically configures the underlying rendering pipeline appropriately.
Key changes:
- Created View class hierarchy (
BaseView,GlobeView,MapView, etc.) to represent different deck.gl views - Added
viewsparameter toMapclass for specifying view instances - Updated default
basemapbehavior to useMaplibreBasemap()instead ofNone - Integrated view type detection to configure Maplibre projection mode and canvas background
Reviewed Changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| lonboard/view.py | Defines new View class hierarchy with FirstPersonView, GlobeView, MapView, OrbitView, and OrthographicView |
| lonboard/_map.py | Adds views parameter and updates basemap default initialization |
| lonboard/types/map.py | Updates MapKwargs TypedDict to include views parameter |
| lonboard/traits.py | Updates ViewStateTrait validation signature to include Map type hint |
| src/model/view.ts | Implements TypeScript view models that build deck.gl view instances |
| src/model/base.ts | Removes unused loadSubModels method |
| src/model/extension.ts | Removes call to removed loadSubModels method |
| src/index.tsx | Integrates view state management and globe-specific styling |
| src/renderers/overlay.tsx | Passes projection="globe" to Maplibre when using GlobeView |
| src/renderers/types.ts | Updates MapRendererProps type to include views |
| src/util.ts | Adds isGlobeView helper function |
| tests/test_map.py | Adds test for default basemap behavior |
| eslint.config.js | Reorganizes import statements alphabetically |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
### Change list
- Adds definitions for map "controls" (which deck.gl calls "widgets",
but the name "widget" is too overloaded in the context of Jupyter, so
I'd like to call them "controls")
- Support for `ScaleControl`, `NavigationControl`, `FullscreenControl`
- These controls are supported by both deck.gl and maplibre, and so
implementations are defined for creating both the deck.gl and maplibre
counterparts
### Example
```py
from lonboard import Map
from lonboard.controls import ScaleControl, FullscreenControl, NavigationControl
from lonboard.basemap import MaplibreBasemap
m = Map([],
basemap=MaplibreBasemap(mode="overlaid"),
controls=[ScaleControl(), FullscreenControl(), NavigationControl()]
)
m
```
<img width="734" height="506" alt="image"
src="https://github.com/user-attachments/assets/42fb5e55-57d8-4b2a-8fe2-f70b804dabce"
/>
We should probably change the default positioning of controls, move the
bbox selector, or make the bbox selector a control itself.
----
However, for deck.gl-driven rendering, I can only see the scale control:
<img width="756" height="611" alt="image"
src="https://github.com/user-attachments/assets/f8e6be57-55ec-45be-9c14-cf76b4ea64d0"
/>
Logged in #990
### Todo list
- Figure out how to pass these react components as children of the
`OverlayRenderer` or `DeckFirstRenderer` (depends on
#921)
- Use the refactored model initialization from
#923 to create the list
of control instances on the deck side
- Integrate more cleanly into the Python API (an extension of
#908)
Closes #842, relevant
to #681
Change list
Map.basemapisMaplibreBasemapif no value ofbasemapwas passed. Add a test for this.projection="globe"to Maplibreviewsparameter toMap. For now this only supports a single view instance, but hopefully in the future it'll support more.This PR also sets the stage for non-geospatial map rendering and for multi-view support.
todo:
viewinstead ofviews? Hopefully soon we'll support multi-views, though maybe not with a maplibre basemap. (Should we name the Map attribute view instead of views? #965 )projection="globe"toviz(Globe parameter intoviz#949)With this PR, to generate a map with globe view, someone would call
Globe view:
Closes #886 (globe view), closes #375 (orthographic view) (though might need more work to allow non-spatial data)
Relates to
ipywidgets#905 (multi-views),